Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: eth_simulateV1 support #10829

Merged
merged 16 commits into from
Sep 18, 2024
Merged

feat: eth_simulateV1 support #10829

merged 16 commits into from
Sep 18, 2024

Conversation

klkvr
Copy link
Collaborator

@klkvr klkvr commented Sep 11, 2024 •
edited
Loading

ref #8281

Adds support for eth_simulateV1, primary using geth implementation as reference (ethereum/go-ethereum#27720).

For now it does not support moving precompiles, and does not calculate resulted block hashes correctly. Precompiles will be added later as it would be a pretty invasive change now.

And regarding block hashes, I am wondering if we have a way to calculate state root for in-memory state? would appreciate some pointers here

Marking as draft as I am still testing it and it's patched to alloy branch with small serde fixes

@mattsse
Copy link
Collaborator

mattsse commented Sep 11, 2024

I am wondering if we have a way to calculate state root for in-memory state?

this should be possible, right @rkrasiuk

reth/crates/rpc/rpc-eth-api/src/helpers/pending_block.rs

Lines 407 to 417 in 026bfcc

let hashed_state = HashedPostState::from_bundle_state(&execution_outcome.state().state);
let receipts_root = self.receipts_root(&block_env, &execution_outcome, block_number);
let logs_bloom =
execution_outcome.block_logs_bloom(block_number).expect("Block is present");
// calculate the state root
let state_provider = &db.database;
let state_root =
state_provider.state_root(hashed_state).map_err(Self::Error::from_eth_err)?;

@rkrasiuk we probably need some help navigating this API, or if this can be used with the CacheDB ?

@rkrasiuk
Copy link
Member

it is possible, all you need is to construct the HashedPostState from state changes. we already do that here

reth/crates/rpc/rpc/src/debug.rs

Lines 640 to 668 in e585436

for (address, account) in db.cache.accounts {
let hashed_address = keccak256(address);
hashed_state.accounts.insert(
hashed_address,
account.account.as_ref().map(|a| a.info.clone().into()),
);
let storage = hashed_state
.storages
.entry(hashed_address)
.or_insert_with(|| HashedStorage::new(account.status.was_destroyed()));
if let Some(account) = account.account {
if include_preimages {
state_preimages
.insert(hashed_address, alloy_rlp::encode(address).into());
}
for (slot, value) in account.storage {
let slot = B256::from(slot);
let hashed_slot = keccak256(slot);
storage.storage.insert(hashed_slot, value);
if include_preimages {
state_preimages.insert(hashed_slot, alloy_rlp::encode(slot).into());
}
}
}
}

one thing that's unclear to me is whether the resulting state root should include state overrides that were passed as inputs

@klkvr
Copy link
Collaborator Author

klkvr commented Sep 11, 2024 •
edited
Loading

one thing that's unclear to me is whether the resulting state root should include state overrides that were passed as inputs

I think it should, given that we persist those between blocks and there might be an edge case when we can't really distinguish between slot that was just overriden and the case when it was overriden, then changed during execution and updated to the same value

mattsse
mattsse requested changes Sep 12, 2024
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this endpoint does a lot -.-

this looks pretty good, the execution logic seems right to me
there are likely a few things missing but we should try to get this in and then once we have a way to get reference data write some tests or fix things

crates/rpc/rpc-eth-api/src/core.rs Outdated
block_number: Option<BlockId>,
) -> RpcResult<Vec<SimulatedBlock>> {
) -> RpcResult<Vec<SimulatedBlock<Block<WithOtherFields<reth_rpc_types::Transaction>>>>> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this not return B, because this response should include txs with additional fields?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our primivie -> rpc conversions return such Block so I've just reused it

reth/crates/rpc/rpc-types-compat/src/block.rs

Lines 18 to 23 in 875cee2

pub fn from_block(
block: BlockWithSenders,
total_difficulty: U256,
kind: BlockTransactionsKind,
block_hash: Option<B256>,
) -> Result<Block<WithOtherFields<Transaction>>, BlockError> {

I think this generic type should be same to the network's block response in general

crates/rpc/rpc-eth-api/src/helpers/call.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-types/src/revm_utils.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-types/src/simulate.rs Outdated Show resolved Hide resolved
@klkvr klkvr mentioned this pull request Sep 12, 2024
Merged
mattsse pushed a commit to paradigmxyz/revm-inspectors that referenced this pull request Sep 12, 2024
@klkvr
feat: add helper for eth_simulateV1 to TransferInspector (#196)
95a25ac 
ref ethereum/execution-apis#484
paradigmxyz/reth#10829

Also added hook for `eofcreate` and moved all handling away from `_env`
hooks to ensure correct ordering.

We don't yet insert logs for selfdestructs, would need update on revm
side to pass journaled state into the hook
@klkvr klkvr marked this pull request as ready for review September 16, 2024 14:48
@klkvr klkvr requested a review from mattsse September 17, 2024 15:56
mattsse
mattsse approved these changes Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty cool.

what's still missing here? the moved precompiles? what else?
could you please add these as todos to the issue?

@klkvr
Copy link
Collaborator Author

klkvr commented Sep 18, 2024

what's still missing here? the moved precompiles? what else?

yeah, only moving precompiles I believe, will track

@klkvr klkvr mentioned this pull request Sep 18, 2024
Closed
@mattsse mattsse changed the title [wip] feat: eth_simulateV1 support feat: eth_simulateV1 support Sep 18, 2024
@mattsse mattsse added this pull request to the merge queue Sep 18, 2024
Merged via the queue into main with commit 91eb50c Sep 18, 2024
36 checks passed
@mattsse mattsse deleted the klkvr/eth-simulatev1 branch September 18, 2024 17:51
@fselmo fselmo mentioned this pull request Sep 20, 2024
Open
lwedge99 pushed a commit to sentioxyz/revm-inspectors that referenced this pull request Oct 8, 2024
@klkvr @lwedge99
feat: add helper for eth_simulateV1 to TransferInspector (paradigmx…
0b0e898 
…yz#196)

ref ethereum/execution-apis#484
paradigmxyz/reth#10829

Also added hook for `eofcreate` and moved all handling away from `_env`
hooks to ensure correct ordering.

We don't yet insert logs for selfdestructs, would need update on revm
side to pass journaled state into the hook
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@emhane emhane Awaiting requested review from emhane emhane is a code owner

@fgimenez fgimenez Awaiting requested review from fgimenez fgimenez is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg onbjerg is a code owner

@gakonst gakonst Awaiting requested review from gakonst gakonst is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@klkvr @mattsse @rkrasiuk